feat(lint): add nursery rule useImportsFirst#9272
feat(lint): add nursery rule useImportsFirst#9272terror wants to merge 6 commits intobiomejs:mainfrom
useImportsFirst#9272Conversation
🦋 Changeset detectedLatest commit: 70dec32 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (2)
WalkthroughAdds a new nursery lint rule Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js (1)
1-5: Optional: add a directive-focused valid fixture.Given the rule docs explicitly allow directives, a tiny
valid_with_directive.js(e.g."use strict";then imports) would lock that behaviour in and guard regressions.Based on learnings: Add tests for all code changes - lint rules need snapshot tests in
tests/specs/{group}/{rule}/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js` around lines 1 - 5, Add a new valid fixture file (e.g., valid_with_directive.js) under tests/specs/nursery/useImportsFirst/ that begins with a directive like "use strict"; followed by the same import lines (import { foo } from "foo"; import { bar } from "bar"; import { baz } from "baz"; const qux = 1;) and the same top comment ("should not generate diagnostics") so the test suite captures that directives are allowed by the useImportsFirst rule; ensure the file follows the existing snapshot test pattern used in tests/specs/{group}/{rule}/ so it is picked up by the rule tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_rule_options/src/use_imports_first.rs`:
- Line 6: Add a rustdoc comment above the public struct UseImportsFirstOptions
explaining what this options type configures (options for the
"use-imports-first" rule/assist), what each field would control (if currently
empty, state that it has no configurable fields yet and is reserved for future
settings), and include example usage or default behavior; ensure the doc uses
standard /// rustdoc format placed immediately before the declaration of
UseImportsFirstOptions.
---
Nitpick comments:
In `@crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js`:
- Around line 1-5: Add a new valid fixture file (e.g., valid_with_directive.js)
under tests/specs/nursery/useImportsFirst/ that begins with a directive like
"use strict"; followed by the same import lines (import { foo } from "foo";
import { bar } from "bar"; import { baz } from "baz"; const qux = 1;) and the
same top comment ("should not generate diagnostics") so the test suite captures
that directives are allowed by the useImportsFirst rule; ensure the file follows
the existing snapshot test pattern used in tests/specs/{group}/{rule}/ so it is
picked up by the rule tests.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid_mixed.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.js.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid_with_export.js.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (7)
crates/biome_js_analyze/src/lint/nursery/use_imports_first.rscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid.jscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/invalid_mixed.jscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid.jscrates/biome_js_analyze/tests/specs/nursery/useImportsFirst/valid_with_export.jscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/use_imports_first.rs
|
dyc3
left a comment
There was a problem hiding this comment.
Looks good, just a couple notes
Merging this PR will not alter performance
Comparing Footnotes
|
Summary
Resolves #9265
This diff Implements the
useImportsFirstlint rule, which enforces that all import statements appear before any non-import statements in a module. This corresponds to the ESLint import/first rule (see #9265). The rule iterates over the module item list and flags anyJsImportthat appears after a non-import item.Test Plan
Snapshot tests in
crates/biome_js_analyze/tests/specs/nursery/useImportsFirst/:invalid.js- imports after a const declaration (2 diagnostics)invalid_mixed.js- imports interspersed with calls and exports (2 diagnostics)valid.js- all imports grouped at the top (no diagnostics)valid_with_export.js- single import before export and statement (no diagnostics)Docs
Rule documentation is inline in the
declare_lint_rule!macro and will be published automatically.